feat(scorecard): File level checks#2751
feat(scorecard): File level checks#2751djanickova wants to merge 23 commits intoredhat-developer:mainfrom
Conversation
|
This pull request adds a new top-level directory under |
Signed-off-by: Diana Janickova <djanicko@redhat.com> test: implement and update unit tests Assisted-By: Cursor Signed-off-by: Diana Janickova <djanicko@redhat.com> chore: generate api reports Signed-off-by: Diana Janickova <djanicko@redhat.com> ref: use metricId instead of providerId Signed-off-by: Diana Janickova <djanicko@redhat.com> ref: Apply suggestions from code review Co-authored-by: Ihor Mykhno <imykhno@redhat.com> fix: run prettier Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: update expected string in test Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com> feat: implement aggregated card and update test Signed-off-by: Diana Janickova <djanicko@redhat.com> feat: include aggregated card in App.tsx Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: pass logger as param Signed-off-by: Diana Janickova <djanicko@redhat.com> ref: remove default boolean tresholds, edit translation Signed-off-by: Diana Janickova <djanicko@redhat.com> chore: generate api reports Signed-off-by: Diana Janickova <djanicko@redhat.com> test: e2e test scenario for file checks Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: selectors in test Signed-off-by: Diana Janickova <djanicko@redhat.com> feat: include file check section in app-config.yaml Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: aggregated permission card title and desc Signed-off-by: Diana Janickova <djanicko@redhat.com> ref: move tresholds from scorecard-common Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: off center icon and treshold imports Signed-off-by: Diana Janickova <djanicko@redhat.com> feat: add new translations Signed-off-by: Diana Janickova <djanicko@redhat.com> ref: implement resolveMetricTranslation Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: resolve translations correctly Signed-off-by: Diana Janickova <djanicko@redhat.com> ref: use useMemo in EmptyStatePanel Signed-off-by: Diana Janickova <djanicko@redhat.com> fix: use translations in test Signed-off-by: Diana Janickova <djanicko@redhat.com>
dd295a2 to
6a60828
Compare
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Review Summary by QodoAdd batch metric providers and GitHub file existence checks
WalkthroughsDescription• Implement batch metric providers supporting multiple metrics per provider • Add GitHub file existence checks (readme, license, codeowners, etc.) • Refactor MetricProvidersRegistry to handle batch and single providers • Add translation support for parameterized metric titles/descriptions • Update UI components to display boolean metrics correctly Diagramflowchart LR
A["MetricProvider Interface"] -->|extends| B["Batch Provider Support"]
B -->|getMetricIds/getMetrics| C["Multiple Metrics"]
B -->|calculateMetrics| D["Batch Calculation"]
E["GithubFilesProvider"] -->|implements| B
E -->|checks| F["File Existence"]
G["MetricProvidersRegistry"] -->|manages| C
G -->|deduplicates| H["Provider Storage"]
I["Translation Utils"] -->|resolves| J["Parameterized Labels"]
K["Scorecard Component"] -->|displays| L["Boolean Metrics"]
File Changes1. workspaces/scorecard/plugins/scorecard-node/src/api/MetricProvider.ts
|
Code Review by Qodo
1.
|
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
Signed-off-by: Diana Janickova <djanicko@redhat.com>
|
Thank you for the PR. I’ve reviewed the code and it looks good. However, during manual testing in my local environment, I noticed that for the Screen.Recording.2026-04-17.at.19.35.14.mov |
| const octokit = await this.getOctokitClient(url); | ||
|
|
||
| const aliasToMetricId = this.buildUniqueAliases([...files.keys()]); | ||
| const fileChecksParts: string[] = []; | ||
|
|
||
| for (const [alias, metricId] of aliasToMetricId) { | ||
| const path = files.get(metricId); | ||
| if (!path) continue; | ||
| const expr = `HEAD:${path}`; | ||
| fileChecksParts.push( | ||
| `${alias}: object(expression: ${JSON.stringify(expr)}) { id }`, | ||
| ); | ||
| } | ||
|
|
||
| const fileChecks = fileChecksParts.join('\n'); | ||
|
|
||
| const query = ` | ||
| query checkFilesExist($owner: String!, $repo: String!) { | ||
| repository(owner: $owner, name: $repo) { | ||
| ${fileChecks} | ||
| } | ||
| } | ||
| `; | ||
|
|
||
| const response = await octokit<{ | ||
| repository: Record<string, { id: string } | null>; | ||
| }>(query, { | ||
| owner: repository.owner, | ||
| repo: repository.repo, | ||
| }); |
There was a problem hiding this comment.
Hi @djanickova, in general this PR is really nice and thanks for all the work you put into this already.
But there is one general (and sorry, its a bigger one) change I like to ask for. The current implementation is a GitHub only implementation but we can expect that customers want check their GitLab, BitBucket etc. repos as well.
Instead of extending the GitHub Scorecard module, I think you should implement a complate Filecheck Scorecard module. And this module should use the ScmIntegration and UrlReaderService instead of Octokit to check if a file exist or not.
I will join the scorecard meeting today for this, let us know if you believe this bigger change is still possible. Sorry that noone mentioned that before.
|
New changes are detected. LGTM label has been removed. |
Signed-off-by: Diana Janickova <djanicko@redhat.com>
|






Hey, I just made a Pull Request!
This PR introduces a new type of scorecard: file level checks (RHDHPLAN-397). This adds ability for Scorecard to verify if a list of required files (one or more) is present (such as a license, CODEOWNERS, dockerfile, .gitignore, etc). The paths to these files are configurable.
Example of these scorecards on the entity page:

Example of these scorecards on homepage:

Example config:
Both app and app-legacy contain example scorecards by default, so you can just test this out by running
yarn start(oryarn start:legacy). You can also edit the files and their paths to try out different scenarios.The contents of this PR have been reviewed in 2 separate previous PRs within my fork:
djanickova#1
djanickova#2
✔️ Checklist